Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add elements to json for bindings to default exchange #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gsker
Copy link

@gsker gsker commented Jun 20, 2014

When generating graphs I was seeing that queues without bindings were being graphed oddly. In my usage of rabbitmq the way they would get messages would be by publishing to the "" exchange with their queue name as the binding key.

This patch adds appropriate data to the json structure to produce that in the graph. I'm not proficient at python, so I don't know how to modify the definitions data in a function other than to make it a global. If you have ideas about the code, let me know. Thanks for the graphs BTW!

@ljcoomber
Copy link
Owner

Hi, sorry for the massive delay but didn't see the notification for this.

Am personally not a fan of using the default bindings as I prefer making routing very explicit. Happy to add the functionality though but would like it:

a) flagged as optional in args

b) comment "That's the normal way a message would get into that queue" removed as I don't think it is

c) factored out a bit. Your Python is fine, but given the complexity of what you're trying to achieve, you could make it clearer using a separate global function along the lines of add_default_bindings, and then use nested functions to make logic explicit

Thanks

@gsker
Copy link
Author

gsker commented Sep 16, 2014

Thanks!
It has been a long time. I totally forgot about it. :-)
I'll make the requested changes and do a new pull request.

I also never use the default bindings intentionally, but I'm not the only one
managing the rabbitmq system so when they are there I want to create a graph
that reflects the routing.

Gerry

On Tue, 16 Sep 2014, Lee Coomber wrote:

Hi, sorry for the massive delay but didn't see the notification for this.

Am personally not a fan of using the default bindings as I prefer making routing
very explicit. Happy to add the functionality though but would like it:

a) flagged as optional in args

b) comment "That's the normal way a message would get into that queue" removed as I
don't think it is

c) factored out a bit. Your Python is fine, but given the complexity of what you're
trying to achieve, you could make it clearer using a separate global function along
the lines of add_default_bindings, and then use nested functions to make logic
explicit

Thanks


Reply to this email directly or view it onGitHub.[949921__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcyNjQ5ODY1NywiZGF
0YSI6eyJpZCI6MzUyMDI5NTd9fQ==--af8cbb7fe5b997bc98571151e6d7ac98943f80db.gif]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants